Skip to content

Climber subsystem version2#4

Open
Gary123abc wants to merge 24 commits intomainfrom
ClimberSubsystem-version2
Open

Climber subsystem version2#4
Gary123abc wants to merge 24 commits intomainfrom
ClimberSubsystem-version2

Conversation

@Gary123abc
Copy link

No description provided.

public ClimberSubsystem() {}
private final SparkMax climberMotor = new SparkMax(5, MotorType.kBrushless);
private final SparkClosedLoopController closedLoopController = climberMotor.getClosedLoopController();
private static final double L1_POSITION = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

參數要放進 Constants 裡面喔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

命令法也要改一下改成小駝峰式

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已改成小駝峰式命名,參數也放入constants了

public static final double minOutput = -1.0;
public static final double maxOutput = 1.0;
public static final int currentLimit = 40;
public static final double L1position = 25.0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這個 L 大寫是可以的,我等下幫你跳過 checkstyle

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'member def modifier' has incorrect indentation level 8, expected level should be 4.
這行是什麼意思
我有好幾條程式碼都跳這個,讓我沒辦法推上去,checkstyle都說有錯

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

就是你的縮排錯了

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

螢幕擷取畫面 2026-01-29 190430 右下角 space 點開 然後 indent using space 改成 2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

然後再重新自動排版(Shift+Alt+F)一下,就會好了

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已經都改好了,checkstyle也有過

Comment on lines 22 to 24
private static final double L1position = ClimberConstants.L1position;
private static final double L2position = ClimberConstants.L2position;
private static final double speed = ClimberConstants.speed;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這個是不是多餘了 可以不用寫

@Hannahjjj97
Copy link
Contributor

阿你如果改完了 要再 request review 一次喔

@Hannahjjj97
Copy link
Contributor

你什麼時候會去實驗室啊
有些東西想要跟你當面講

@Hannahjjj97
Copy link
Contributor

啊你可以先寫簡易版 command 了

@Gary123abc
Copy link
Author

我應該禮拜日才會去

@Gary123abc
Copy link
Author

欸等等禮拜日要消毒,那我禮拜六應該會去,只不過就只能去早上,下午有補習

Copy link
Contributor

@Hannahjjj97 Hannahjjj97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

先加一下簡易版 Command 喔

@Hannahjjj97
Copy link
Contributor

欸等等禮拜日要消毒,那我禮拜六應該會去,只不過就只能去早上,下午有補習

好 那我星期六早上去找你?

@Hannahjjj97
Copy link
Contributor

排版修完之後,monday 可以先 done,然後剩下的部分星期六再來處理。

@Gary123abc
Copy link
Author

Type name 'climberCommand' must match pattern '^[A-Z][a-zA-Z0-9]*$'.是什麼意思?
對了可以順便問問禮拜六是要去做什麼嗎

@Hannahjjj97
Copy link
Contributor

Type name 'climberCommand' must match pattern '^[A-Z][a-zA-Z0-9]*$'.是什麼意思?

對了可以順便問問禮拜六是要去做什麼嗎

星期六我會跟你說程式要怎麼改,然後如果來得及改的話可以拿馬達測邏輯,就是他能不能在該停的地方停下來。

@Hannahjjj97
Copy link
Contributor

Type name 'climberCommand' must match pattern '^[A-Z][a-zA-Z0-9]*$'.是什麼意思?

對了可以順便問問禮拜六是要去做什麼嗎

要大寫開頭

@BrianHu0925
Copy link

好像沒看到 merge main 的 commit?

Copy link
Member

@kennhung kennhung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

除了有註解的部分之外,RobotContainer 不應該在這個分支被改到。

另外,每個 ClimberSybsystem 裡面每個 Command 的行為好像都有點不太合理。(runEnd 的 end 動作)

SparkMaxConfig config = new SparkMaxConfig();
config
.idleMode(com.revrobotics.spark.config.SparkBaseConfig.IdleMode.kBrake)
.smartCurrentLimit(ClimberConstants.currentLimit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

為什麼需要特別調整 SparkMax 預設的 CurrentLimit?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gary123abc 可以回覆一下嗎?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因為原本有要測馬達邏輯,所以就先設小一點,然後clamp的數值也是

@Gary123abc Gary123abc requested a review from kennhung February 4, 2026 13:01
Copy link
Member

@kennhung kennhung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

上次跟這次 review 有留了一些問題,可以再回覆一下嗎?

return Commands.print("No autonomous command configured");
}
}
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

啊,你這邊好像又不小心把一行刪掉了
你可以看 PR 的 Files changed 頁面確認一下跟原本的差在哪

public void periodic() {
// This method will be called once per scheduler run

double measurement = MathUtil.clamp(climberEncoder.get(), -1.0, 1.0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

為什麼會需要把 encoder 讀到的值 clamp 在 -1 ~ 1 之間啊?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

應該是motor(set)設clamp嗎

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看你想要做的是什麼事情?
clamp 在 motor.set 就會把馬達輸出限定在一個範圍內
同理,用在 encoder.get 就會把 encoder 輸出限定在一個範圍內

Comment on lines 58 to 62
public void resetEncoder() {
climberMotor.getEncoder().setPosition(0);
climberPID.reset();
climberPID.setSetpoint(0);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感覺有點怪怪的 你爬升用的是 dutycycle encoder 可是這裡 reset 的是 sparkMax 的

Copy link
Member

@kennhung kennhung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RobotContainer.java 還是跟 main 不同,有被改到

Copy link
Member

@kennhung kennhung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RobotContainer.java 還是跟 main 不同,有被改到
另外,有其中一個留言有問問題,請回覆一下,不要直接設為已解決

}

@Override
public void periodic() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

希望可以在不同的東西之間加上空行,會比較方便其他人閱讀。(可以分成取得目前位置、控制馬達、把東西放上 Dashboard)

SparkMaxConfig config = new SparkMaxConfig();
config
.idleMode(com.revrobotics.spark.config.SparkBaseConfig.IdleMode.kBrake)
.smartCurrentLimit(ClimberConstants.currentLimit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gary123abc 可以回覆一下嗎?

private final PIDController climberPID = new PIDController(
ClimberConstants.kP, ClimberConstants.kI, ClimberConstants.kD);
private final DutyCycleEncoder climberEncoder = new DutyCycleEncoder(ClimberConstants.encoderChannel, 360, 0);
private final DigitalInput manualSwitch = new DigitalInput(ClimberConstants.manualSwitchChannel);
Copy link
Contributor

@Hannahjjj97 Hannahjjj97 Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是按鈕,所以應該是要用 Supplier<Boolean>

Comment on lines +130 to +134
if (climberPID.atSetpoint()) {
climberMotor.set(0);
} else {
climberMotor.set(limitedOutput);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這樣寫感覺可能會抖?

Comment on lines +96 to +105
public Command climbUpCmd() {
Command cmd = this.run(() -> climbUp()).finallyDo(() -> climberMotor.set(0));
cmd.setName("climbUpCmd");
return cmd;
}

public Command climbDownCmd() {
Command cmd = this.run(() -> climbDown()).finallyDo(() -> climberMotor.set(0));
cmd.setName("climbDownCmd");
return cmd;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這樣寫感覺會爆衝欸?

Comment on lines +55 to +69
public void climbUp() {
if (manualModeSupplier.getAsBoolean()) {
climberMotor.set(0.3);
} else {
climberPID.setSetpoint(climberEncoder.get() + 10);
}

}

private void toMidRung() {
public void climbDown() {
if (manualModeSupplier.getAsBoolean()) {
climberMotor.set(-0.3);
} else {
climberPID.setSetpoint(climberEncoder.get() - 10);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這些要不要拆成共四個 command 啊
直接給 power 上下的兩個
設定 setpoint 的兩個
到時候寫按鈕的時候再把判斷指撥開關有沒有開的判斷加上去

@Hannahjjj97
Copy link
Contributor

現在這個寫法在從直接給馬達輸出切換成 pid 控制的時候會不會有問題啊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants